Skip to content

Conversation

gamesh411
Copy link
Contributor

The check was crashing when trying to evaluate value-dependent expressions using EvaluateAsInt() in cases where the src parameter of memcpy is value-dependent, but the length is not. Added isValueDependent() check before EvaluateAsInt() call to prevent the crash.

The check was crashing when trying to evaluate value-dependent expressions using
EvaluateAsInt() in cases where the src parameter of memcpy is value-dependent,
but the length is not. Added isValueDependent() check before EvaluateAsInt()
call to prevent the crash.
@llvmbot
Copy link
Member

llvmbot commented Sep 25, 2025

@llvm/pr-subscribers-clang-tools-extra

@llvm/pr-subscribers-clang-tidy

Author: Endre Fülöp (gamesh411)

Changes

The check was crashing when trying to evaluate value-dependent expressions using EvaluateAsInt() in cases where the src parameter of memcpy is value-dependent, but the length is not. Added isValueDependent() check before EvaluateAsInt() call to prevent the crash.


Full diff: https://github.com/llvm/llvm-project/pull/160727.diff

2 Files Affected:

  • (modified) clang-tools-extra/clang-tidy/bugprone/NotNullTerminatedResultCheck.cpp (+5-3)
  • (added) clang-tools-extra/test/clang-tidy/checkers/bugprone/not-null-terminated-result-value-dependent-crash.cpp (+23)
diff --git a/clang-tools-extra/clang-tidy/bugprone/NotNullTerminatedResultCheck.cpp b/clang-tools-extra/clang-tidy/bugprone/NotNullTerminatedResultCheck.cpp
index d4676842a97ff..463677d2d3af6 100644
--- a/clang-tools-extra/clang-tidy/bugprone/NotNullTerminatedResultCheck.cpp
+++ b/clang-tools-extra/clang-tidy/bugprone/NotNullTerminatedResultCheck.cpp
@@ -64,15 +64,17 @@ static unsigned getLength(const Expr *E,
   if (!E)
     return 0;
 
-  Expr::EvalResult Length;
   E = E->IgnoreImpCasts();
 
   if (const auto *LengthDRE = dyn_cast<DeclRefExpr>(E))
     if (const auto *LengthVD = dyn_cast<VarDecl>(LengthDRE->getDecl()))
       if (!isa<ParmVarDecl>(LengthVD))
         if (const Expr *LengthInit = LengthVD->getInit())
-          if (LengthInit->EvaluateAsInt(Length, *Result.Context))
-            return Length.Val.getInt().getZExtValue();
+          if (!LengthInit->isValueDependent()) {
+            Expr::EvalResult Length;
+            if (LengthInit->EvaluateAsInt(Length, *Result.Context))
+              return Length.Val.getInt().getZExtValue();
+          }
 
   if (const auto *LengthIL = dyn_cast<IntegerLiteral>(E))
     return LengthIL->getValue().getZExtValue();
diff --git a/clang-tools-extra/test/clang-tidy/checkers/bugprone/not-null-terminated-result-value-dependent-crash.cpp b/clang-tools-extra/test/clang-tidy/checkers/bugprone/not-null-terminated-result-value-dependent-crash.cpp
new file mode 100644
index 0000000000000..5f361c35e448c
--- /dev/null
+++ b/clang-tools-extra/test/clang-tidy/checkers/bugprone/not-null-terminated-result-value-dependent-crash.cpp
@@ -0,0 +1,23 @@
+// RUN: %check_clang_tidy %s bugprone-not-null-terminated-result %t -- \
+// RUN: -- -std=c++17 -I %S/Inputs/not-null-terminated-result
+
+// This test case reproduces the crash when the check tries to evaluate
+// a value-dependent expression using EvaluateAsInt() in
+// bugprone-not-null-terminated-result, where the src parameter of memcpy is
+// value-dependent, but the length is not.
+
+// expected-no-diagnostics
+
+#include "not-null-terminated-result-cxx.h"
+
+template<size_t N>
+class ValueDependentClass {
+public:
+  void copyData(char* Dst) {
+    const char* Src = reinterpret_cast<const char*>(this);
+    // The length parameter is arbitrary, but the crash is not reproduced if it is N.
+    memcpy(Dst, Src, 32);
+  }
+};
+
+template class ValueDependentClass<42>; // The template parameter value is arbitrary.

Copy link
Contributor

@vbvictor vbvictor left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please add release note in clang-tools-extra/ReleaseNotes.rst.
Make entry in lexical order

Comment on lines 72 to +73
if (const Expr *LengthInit = LengthVD->getInit())
if (LengthInit->EvaluateAsInt(Length, *Result.Context))
return Length.Val.getInt().getZExtValue();
if (!LengthInit->isValueDependent()) {
Copy link
Contributor

@HerrCai0907 HerrCai0907 Sep 26, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

if (const Expr *LengthInit = LengthVD->getInit(); LengthInit && !LengthInit->isValueDependent()) {

Copy link
Contributor

@HerrCai0907 HerrCai0907 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM
please add release note also

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants